samples: Bluetooth: Broadcast multiple legacy and extended advertising #94304
Conversation
cvinayak
force-pushed
the
github_broadcast_multiple_with_legacy
branch
9 times, most recently
from
771caba to
7afe802
Compare
zephyrbot
added
platform: nRF BSIM
zephyrbot
requested review from
HaavardRei,
alwa-nordic,
asbjornsabo,
fredrikdanebjer,
hermabe,
jhedberg,
jthm-ot,
kartben,
larsgk,
nashif and
niym-ot
cvinayak
force-pushed
the
github_broadcast_multiple_with_legacy
branch
2 times, most recently
from
427feb4 to
f815c12
Compare
There was a problem hiding this comment.
Rename this file to bt_ll_sw_split.conf
cc @nordicjm
There was a problem hiding this comment.
Rename this file to bt_ll_sw_split.conf
cc @nordicjm
There was a problem hiding this comment.
Rename this file to bt_ll_sw_split-all.conf
cc @nordicjm
There was a problem hiding this comment.
Rename this file to bt_ll_sw_split-all.conf
cc @nordicjm
There was a problem hiding this comment.
Rename this file to bt_ll_sw_split.conf
cc @nordicjm
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| /* Use advertising set instance index as SID */ | ||
| adv_param.sid = index; | ||
|
|
||
| /* Advertising set options, AD and AD array size */ |
There was a problem hiding this comment.
Using modulo operation to cycle through configurations could be confusing when CONFIG_BT_EXT_ADV_MAX_ADV_SET is not a multiple of param_config array size. Consider adding a comment explaining this behavior or bounds checking.
| /* Advertising set options, AD and AD array size */ | |
| /* Advertising set options, AD and AD array size */ | |
| /* | |
| * Cycle through param_config using modulo operation. | |
| * If CONFIG_BT_EXT_ADV_MAX_ADV_SET is greater than ARRAY_SIZE(param_config), | |
| * configurations will repeat for additional advertising sets. | |
| * This is intentional; update param_config or CONFIG_BT_EXT_ADV_MAX_ADV_SET | |
| * if a one-to-one mapping is required. | |
| */ |
| LL_ASSERT_MSG((ticks_elapsed <= ticks_to_start), "%s overhead = %u (%u) us.", | ||
| __func__, HAL_TICKER_TICKS_TO_US(ticks_elapsed), | ||
| HAL_TICKER_TICKS_TO_US(ticks_to_start)); |
There was a problem hiding this comment.
The assertion message format string expects 3 arguments but only provides 2 after %s. This will cause undefined behavior in the assertion message formatting.
cvinayak
force-pushed
the
github_broadcast_multiple_with_legacy
branch
2 times, most recently
from
56d6b00 to
1209301
Compare
Enable similar Controller Kconfig features between nRF52 and nRF5340 BabbleSIM LE Audio testing. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * scanning instances can expire changing the ticks_current value while we are querying for | ||
| * aux_offset value. | ||
| */ | ||
| #define MAX_RETRY_TICKS_CURRENT_CHANGE ((CONFIG_BT_CTLR_ADV_AUX_SET) - 1U + 2U) |
There was a problem hiding this comment.
[nitpick] Extra space in the macro definition. There are two spaces between -1U and +, should be one for consistency.
| #define MAX_RETRY_TICKS_CURRENT_CHANGE ((CONFIG_BT_CTLR_ADV_AUX_SET) - 1U + 2U) | |
| #define MAX_RETRY_TICKS_CURRENT_CHANGE ((CONFIG_BT_CTLR_ADV_AUX_SET) - 1U + 2U) |
| @@ -0,0 +1,30 @@ | |||
| # Increased to 4 gives one each of legacy, 2M, 1M and Coded PHY advertising sets | |||
There was a problem hiding this comment.
Typo in comment: "gives" should be "give" (subject-verb agreement with "Increased to 4").
| # Increased to 4 gives one each of legacy, 2M, 1M and Coded PHY advertising sets | |
| # Increased to 4 give one each of legacy, 2M, 1M and Coded PHY advertising sets |
Tune the aux offset calculation overhead assertion to use EVENT_OVERHEAD_START_US; this is the correct maximum overhead causing ISR latency for start of a radio event that occurs thereafter. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
cvinayak
force-pushed
the
github_broadcast_multiple_with_legacy
branch
2 times, most recently
from
013f0a6 to
f617a43
Compare
Fix missing auxiliary chain pdu time reservation. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Update broadcaster_multiple sample to start multiple advertising sets of type legacy and extended advertising. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Move Zephyr Controller Kconfigs to overlay file. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Move harness and tags to common section for observer and broadcaster_multiple samples. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Do not use the BT_ prefix for definitions not provided by the Bluetooth subsystem. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
cvinayak
force-pushed
the
github_broadcast_multiple_with_legacy
branch
from
f617a43 to
08574e2
Compare
|
6 participants
Update broadcaster_multiple sample to start multiple advertising sets of type legacy and extended advertising.
How they appear on-air (BabbleSim import), for 4 advertising sets:
